Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ipachart] 0.2 initial public test release #2845

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

rowbory
Copy link
Contributor

@rowbory rowbory commented Jun 25, 2024

Has anyone tried making a touch IPA chart before?

@keyman-server
Copy link
Collaborator

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

@rowbory rowbory changed the title 0.2 initial public test release [ipachart] 0.2 initial public test release Jun 25, 2024
@LornaSIL
Copy link
Contributor

There is a touch layout for the sil_ipa keyboard, but it isn't well documented.

For this keyboard, it looks like you may have started with your naijatype files and manually edited them? You'll need to clean up all references to folders, etc for naijatype. I saw some in your .kps file.

NaijaType references removed and links checked. Touch tone outputs were wrong.
@keyman-server
Copy link
Collaborator

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

@rowbory
Copy link
Contributor Author

rowbory commented Jun 25, 2024

There is a touch layout for the sil_ipa keyboard, but it isn't well documented.

For this keyboard, it looks like you may have started with your naijatype files and manually edited them? You'll need to clean up all references to folders, etc for naijatype. I saw some in your .kps file.

Oops. Yes, I think that's left over from 4 years ago when I started it. Thanks for spotting it. It was just the Package because the IPA chart itself I started from a template. sil_ipa is based on a QWERTY keyboard and this keyboard is based on the IPA chart.

@darcywong00
Copy link
Contributor

There is a touch layout for the sil_ipa keyboard, but it isn't well documented.

We'd like to address that issue #2213
which will depend on a new Keyman Developer module in keymanapp/keyman#11792

<OSKFont>Andika-Regular.ttf</OSKFont>
<DisplayFont>Andika-Regular.ttf</DisplayFont>
<Languages>
<Language ID="zzz">IPA</Language>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should use the same language ID as sil_ipa

<Language ID="und-Latn">und-Latn</Language>

but depends on keymanapp/keyman#11046 gets resolved.

@darcywong00
Copy link
Contributor

Can these files be removed?

  • experimental/i/ipachart/ipachart_code_generator.ods (binary file)
  • experimental/i/ipachart/source/ipachart-codes.txt (empty file)

@keyman-server
Copy link
Collaborator

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

@rowbory
Copy link
Contributor Author

rowbory commented Jun 27, 2024

Can these files be removed?

  • experimental/i/ipachart/ipachart_code_generator.ods (binary file)
  • experimental/i/ipachart/source/ipachart-codes.txt (empty file)

Yes. Done. The ipachart_code_generator is useful for generating some of the CSS and KMN code, but I'll keep it in a private repo.

@keyman-server
Copy link
Collaborator

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

@keyman-server
Copy link
Collaborator

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

@keyman-server
Copy link
Collaborator

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

@mcdurdin
Copy link
Member

Hmm, I would be happy for the .ods file to remain, even though it is binary. Good for future maintainers.

@LornaSIL
Copy link
Contributor

LornaSIL commented Jul 1, 2024

Some feedback:

  • Hopefully you are using Keyman 17.
  • Delete: ipachart.keyboard_info and make sure all that info is now in the .kps file
  • Delete LICENSE and keep LICENSE.md
  • Remove version and copyright years from README.md. This helps future maintenance.
  • welcome.htm and readme.htm - remove copyright year.
  • You don't have a source\help\ipachart.php file. While not required for keyboards in experimental it would be nice.
  • You have ipachart-bg.jpg but I don't see it being used anywhere. You should either delete it (and delete from .kps)
  • I'm not sure how the targets can be "any". There are hardly any rules in the keyboard except for lines 14-23. All the rest are for a touch layout.
    • And even then lines 15 and 17 say they will never be fired. The rule will never be matched for key 'ʃ' because its key code is never fired.
  • These 3 rules are putting out U+0329 both before and after the character. I'm not sure if that is what you want. Since you are using the glyphs rather than the codepoints, you can't see it in the code:
+ [T_n_syllabic] > '̩n̩'
+ [T_ng_syllabic] > '̩ŋ̩'
+ [T_ny_syllabic] > '̩ɳ̩'
  • There is a .kvks file that you have committed, but the .kmn doesn't reference it. Either reference the .kvks or delete the file from the PR.
  • We're getting a bit confused about what the langtag should be for the IPA keyboards. Before we moved to Keyman 17 build system we had two language tags, one in the keyboard_info (und-fonipa) file and one in the .kps (und-Latn). Now we don't have a keyboard_info and we are down to using und-Latn in the .kps. I'm sorry for the confusing feedback you are getting. (Windows can't handle und-fonipa.

You may, if you want to, include these two files:

    experimental/i/ipachart/ipachart_code_generator.ods (binary file)
    experimental/i/ipachart/source/ipachart-codes.txt (empty file)

Our practice has generally been to create an assets folder so they are there for future use, but not in the same folder as the keyboards so it doesn't get confused. You could use experimental/i/ipachart/assets

@keyman-server
Copy link
Collaborator

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

@DavidLRowe
Copy link
Contributor

Currently this keyboard seems to be targeting mobile only. The "Supported Platforms" section of the README.md should change accordingly, along with the "&TARGETS" store in the .kmn file, which could be 'tablet phone' (or perhaps 'tablet phone web') rather than 'any'.

In the past there have been complaints about a "mobile only" keyboard ceasing to work when an external keyboard was plugged in. It would be helpful if your documentation indicates that the keyboard won't work with an external keyboard attached to the device. (Or else, add a desktop keyboard, of course!)

There are compiler warnings indicating that these two lines will never be used, since the keystroke (between the "+" and the ">") doesn't correspond to a keyboard key:
't' + 'ʃ' > 't͡ʃ'
'd' + 'ʒ' > 'd͡ʒ'

There's also a warning that flick or multi-tap gestures need version 17 of the compiler (that is, a
store(&Version) "17.0"
statement in the .kmn file.

@rowbory
Copy link
Contributor Author

rowbory commented Jul 3, 2024 via email

@DavidLRowe
Copy link
Contributor

@rowbory No pressure! Just wanting to make sure that you're not waiting on anything from us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

6 participants